[incubator-kie-drools-6180] accumulate min doesn't evaluate correctly… #6186
Conversation
… with more than 18 digits BigDecimal
tkobayas
force-pushed
the
incubator-kie-drools-6180-minBD
branch
from
1a431a7 to
14db10e
Compare
| "then\n" + | ||
| " results.add($min);\n" + | ||
| // to confirm if $min is BigInteger at build time (Not Comparable) | ||
| // The return value isn't important to assert |
There was a problem hiding this comment.
Are these comments relevant? it seems that we are asserting the result values.
There was a problem hiding this comment.
The comment is to explain the below code " System.out.println($min.nextProbablePrime());\n" . I'll rewrite it better. Thanks.
| " results.add($min);\n" + | ||
| // to confirm if $min is BigInteger at build time (Not Comparable) | ||
| // The return value isn't important to assert | ||
| " System.out.println($min.nextProbablePrime());\n" + |
There was a problem hiding this comment.
Do we need to printout this value? if possible a test should not print values, only check them in assertions.
There was a problem hiding this comment.
nextProbablePrime() is specific to BigInteger class. So, this line is to check that drl build doesn't raise a compile error, so we can confirm that $min is treated as BigInteger in the internally generated java code. The return value of nextProbablePrime() is not guaranteed, so we cannot assert the value is not important, I didn't assert.
Having said that, I can avoid System.out.println anyway. I'll fix the PR. Thanks.
|
re-running GHA |
|
Test failures on CallbackStateTimeoutsIT, MultipleTimerInstancesIT etc. are being solved by apache/incubator-kie-kogito-runtimes#3814 |
apache#6186) * [incubator-kie-drools-6180] accumulate min doesn't evaluate correctly with more than 18 digits BigDecimal * fixed assertion
… with more than 18 digits BigDecimal
Issue:
At first, I thought I can use
MinAccumulateFunctionforBigDecimalandBigInteger. However, itsgetResultType()returnsComparable.class, so it causes a compile error when using methods like$min.intValue(). We may change the strategy to cache accumulate function instances (currently, per function name) to makeMinAccumulateFunctionwork for various types, but I followed the current approach which is easier to understand, so I added 4 function classesBigDecimalMinAccumulateFunction(minBD),BigIntegerMinAccumulateFunction(minBI),BigDecimalMaxAccumulateFunction(maxBD),BigIntegerMaxAccumulateFunction(maxBI)